feat: add srcset generation #25

sherwinski · 2019-09-19T03:37:35Z

This PR creates a new class method in UrlBuilder that will generate a srcset string, which can be used in the srcset attribute on an <img> HTML element. BuildSrcSet() takes a path and params argument similar to BuildUrl(), and will return a String in one of two srcset formats.

Srcset Width-Pairs
The first format creates srcset width-pairs, a comma-delimited list of URLs and width descriptors corresponding to each one. This allows for responsive size switching based on the current width of the browser viewport. BuildSrcSet() will append any params passed to the method to each URL constructed within the list. The following is an example of how to generate a width-pair srcset:

UrlBuilder builder = new UrlBuilder("test.imgix.net", signKey:"", includeLibraryParam: false, useHttps: true);
Dictionary<String, String> parameters = new Dictionary<string, string>();
parameters["fit"] = "crop";            
parameters["crop"] = "faces";
String srcset = builder.BuildSrcSet("bridge.png", parameters);
Console.WriteLine(srcset);

will return the following String (collapsed for brevity):

https://test.imgix.net/bridge.png?fit=crop&crop=faces&w=100 100w,
https://test.imgix.net/bridge.png?fit=crop&crop=faces&w=116 116w,
https://test.imgix.net/bridge.png?fit=crop&crop=faces&w=134 134w,
								...
https://test.imgix.net/bridge.png?fit=crop&crop=faces&w=7400 7400w,
https://test.imgix.net/bridge.png?fit=crop&crop=faces&w=8192 8192w

Notice that a w= parameter is automatically inserted for each entry in the srcset list. This is required in order for the element to properly display the correctly-sized image corresponding to the viewport's width.

Device Pixel Ratio (DPR) Srcset
The second format this method can return is a srcset list of same-size images in varying resolutions. In this case, images are scaled using the dpr= parameter to adjust for the device pixel ratio of the browser. A DPR srcset will be automatically generated instead of a width-pair srcset if exact dimensions for the output image are specified, by providing either a w (width) or a h (height) and ar (aspect ratio) in the parametes argument.
The following is an example of how to generate a DPR srcset:

UrlBuilder builder = new UrlBuilder("test.imgix.net", signKey:"", includeLibraryParam: false, useHttps: true);
Dictionary<String, String> parameters = new Dictionary<string, string>();
parameters["h"] = "200";
parameters["ar"] = "3:2";   
String srcset = builder.BuildSrcSet("bridge.png", parameters);
Console.WriteLine(srcset);

which will return the following String:

https://test.imgix.net/bridge.png?h=200&ar=3%3A2&dpr=1 1x,
https://test.imgix.net/bridge.png?h=200&ar=3%3A2&dpr=2 2x,
https://test.imgix.net/bridge.png?h=200&ar=3%3A2&dpr=3 3x,
https://test.imgix.net/bridge.png?h=200&ar=3%3A2&dpr=4 4x,
https://test.imgix.net/bridge.png?h=200&ar=3%3A2&dpr=5 5x

Signing URLs
BuildSrcSet() also supports signing URLs, which can be very useful for generating server-side and then passing to the client. This will avoid having to expose your imgix source's secure token to the client.

UrlBuilder builder = new UrlBuilder("test.imgix.net", signKey:"my-token", includeLibraryParam: false, useHttps: true);
Dictionary<String, String> parameters = new Dictionary<string, string>();
parameters["w"] = "800";
String srcset = builder.BuildSrcSet("bridge.png", parameters);   
Console.WriteLine(srcset);

generates a srcset with unique signatures for each URL:

https://test.imgix.net/bridge.png?w=800&dpr=1&s=48ab8bc59b52eaa6145df25139742dde 1x,
https://test.imgix.net/bridge.png?w=800&dpr=2&s=6d9e5473aac1deab62f91d940c5e3f42 2x,
https://test.imgix.net/bridge.png?w=800&dpr=3&s=c37997036878f97a9c42f29548bb6856 3x,
https://test.imgix.net/bridge.png?w=800&dpr=4&s=10606b93fa955a3e310ba5564627b4ea 4x,
https://test.imgix.net/bridge.png?w=800&dpr=5&s=2b7eb1ef729e7a794d857681b51f1c0e 5x

For more information on srcset, building responsive images, and resolution switching:

…m twice

sherwinski · 2019-09-26T19:32:13Z

src/Imgix/UrlBuilder.cs

+            foreach(int ratio in targetRatios)
+            {
+                parameters["dpr"] = ratio.ToString();
+                parameters.Remove("ixlib");


@zacman85 Looks like the way we're adding ixlib (L57) is causing a collision error here unless we "remove" it between calls to BuildUrl. Curious if you think this strategy is preferable over say, changing L57 to use parameters["ixlib"] = ..., which will avoid the same issue but requires touching on the original BuildUrl implementation.

In general, I do not like to have to remove things from a list like this, when something is being built up, unless it is unavoidable. I don't know enough about the codebase to suggest one way over the other as I don't know the consequences of a refactor here. I would leave it to your judgement.

The immediate issue here is that BuildUrl indiscriminately Adds an ixlib parameter if the flag has been set. The real issues are more nuanced...

Nonetheless, Remove can be removed from the codebase if we just check if the parameters(the dictionary/hash-map that is passed) already contain the ixlib key-value pair. If so, continue; otherwise, if the flag is set, but no ixlib param is present, Add it.

I would also go a bit further than this:

In general, I do not like to have to remove things from a list like this, when something is being built up, unless it is unavoidable.

We're at the build step. All configuration should have stopped by this point.

The builder pattern is a great way of separating "configuration" from "behavior." For instance, we should start thinking about what these libraries do as a three step process:

initialize

configure

construct

Initialization describes the purpose of language-level constructors. Configuration describes the steps taken to configure the data within the initialized data structure (object, etc.). Construction describes the process of "putting it all together." Note that this last step boils down to concatenating or joining the configuration data.

The problem with functions like BuildUrl is that they abuse abstraction barriers. Here, this function does a little configuration then a little construction (or delegation to some function that ultimately constructs the result). Calling Build here should be the final step. No mutation needs to occur to any of the configuration values.

zacman85 · 2019-09-27T18:04:37Z

src/Imgix/UrlBuilder.cs

+            foreach(int ratio in targetRatios)
+            {
+                parameters["dpr"] = ratio.ToString();
+                parameters.Remove("ixlib");


In general, I do not like to have to remove things from a list like this, when something is being built up, unless it is unavoidable. I don't know enough about the codebase to suggest one way over the other as I don't know the consequences of a refactor here. I would leave it to your judgement.

Sherwin H added 8 commits September 18, 2019 23:37

add targetWidths()

8128785

feat: add GenerateSrcSet() method to UrlBuilder

14ab5fb

test: add srcset tests

34d6c2b

docs: add srcset documentation

5db6f3f

add dpr parameter to DPR srcset; handle conflict in adding ixlib para…

5e3d977

…m twice

test: fix broken tests

7f46399

docs: fix typos

8d4a358

refactor: pull srcset width calculations into static class variable

362f538

sherwinski requested a review from zacman85 September 26, 2019 19:15

sherwinski commented Sep 26, 2019

View reviewed changes

zacman85 approved these changes Sep 27, 2019

View reviewed changes

jayeb approved these changes Sep 27, 2019

View reviewed changes

sherwinski merged commit f66ae42 into master Sep 27, 2019

sherwinski deleted the srcset-generation branch September 27, 2019 23:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add srcset generation #25

feat: add srcset generation #25

sherwinski commented Sep 19, 2019 •

edited

Loading

sherwinski Sep 26, 2019

zacman85 Sep 27, 2019

ericdeansanchez May 5, 2020

ericdeansanchez May 5, 2020 •

edited

Loading

zacman85 Sep 27, 2019

feat: add srcset generation #25

feat: add srcset generation #25

Conversation

sherwinski commented Sep 19, 2019 • edited Loading

sherwinski Sep 26, 2019

Choose a reason for hiding this comment

zacman85 Sep 27, 2019

Choose a reason for hiding this comment

ericdeansanchez May 5, 2020

Choose a reason for hiding this comment

ericdeansanchez May 5, 2020 • edited Loading

Choose a reason for hiding this comment

zacman85 Sep 27, 2019

Choose a reason for hiding this comment

sherwinski commented Sep 19, 2019 •

edited

Loading

ericdeansanchez May 5, 2020 •

edited

Loading